chore: bump version_id changeset from patch to minor#29
Merged
ErnestM1234 merged 1 commit intogeneraltranslation:mainfrom Mar 19, 2026
Merged
Conversation
| "cacheUrl": "cache_url", | ||
| "runtimeUrl": "runtime_url", | ||
| "customMapping": "custom_mapping", | ||
| "_versionId": "version_id", |
There was a problem hiding this comment.
Unconventional leading-underscore config key
The JSON config key is mapped as "_versionId" (with a leading underscore). All other keys in _KEY_MAP follow standard camelCase without a leading underscore ("projectId", "cacheUrl", "customMapping", etc.).
If this leading underscore is intentional (e.g. to signal that it is an internal/private field in gt.config.json), a brief comment would help future readers understand the convention. If it is unintentional, it should be renamed to "versionId" for consistency.
Suggested change
| "_versionId": "version_id", | |
| "_versionId": "version_id", # leading underscore intentional — marks as internal-use field |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/gt-i18n/src/gt_i18n/internal/_load_gt_config.py
Line: 19
Comment:
**Unconventional leading-underscore config key**
The JSON config key is mapped as `"_versionId"` (with a leading underscore). All other keys in `_KEY_MAP` follow standard camelCase without a leading underscore (`"projectId"`, `"cacheUrl"`, `"customMapping"`, etc.).
If this leading underscore is intentional (e.g. to signal that it is an internal/private field in `gt.config.json`), a brief comment would help future readers understand the convention. If it is unintentional, it should be renamed to `"versionId"` for consistency.
```suggestion
"_versionId": "version_id", # leading underscore intentional — marks as internal-use field
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
abac13e to
3532138
Compare
ErnestM1234
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The version_id feature (get_version_id() + version_id param for initialize_gt()) was mistakenly tagged as a patch bump. This updates the changeset to minor for gt-i18n, gt-fastapi, and gt-flask.
Greptile Summary
This PR introduces
version_idsupport acrossgt-i18n,gt-fastapi, andgt-flask— adding aversion_idparameter toinitialize_gt(), storing it inI18nManager, exposing it via a newget_version_id()helper, and mapping it from_versionIdingt.config.json— then corrects the changeset bump frompatchtominorto match the scope of the feature addition.version_idstored but not used in CDN translation loading: The parameter is accepted, resolved from config/args, and stored inI18nManager, butcreate_remote_translation_loader()is never givenversion_id, so the built-in CDN URL remains{cache_url}/{project_id}/{locale}. Users expecting the flag to "pin translations" (as the docstring states) will observe no difference in behaviour when using the default CDN loader._versionIdkey: Thegt.config.jsonkey uses a leading underscore, unlike every other key in_KEY_MAP. If this is intentional (marking it as an internal/private field) a comment would clarify intent; otherwise it should be"versionId"for consistency.gt-fastapiandgt-flask(param → resolve → pass to manager) is symmetric and correct.helpers/_version_id.pyfollows the same pattern as the existinghelpers/_locales.py.Confidence Score: 3/5
version_idis stored but has no effect on translation loading.version_idis never forwarded to the remote CDN loader, meaning the feature silently does nothing when using the default loader. This is a functional gap that should be resolved or clearly documented before shipping.packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.pyandpackages/gt-i18n/src/gt_i18n/i18n_manager/_remote_loader.pyneed attention to wireversion_idinto the CDN URL.Important Files Changed
version_idparam toI18nManager.__init__and aget_version_id()getter, but the stored value is never forwarded to the CDN remote loader, so it has no actual effect on which translations are fetched.get_version_id()to the manager singleton; follows the same pattern as_locales.py._versionId(note: leading underscore, unlike every other key) toversion_idin the config; also addsversion_idto theGTConfigTypedDict.version_idparam toinitialize_gt(), resolves it against the file config, and passes it toI18nManager. Implementation is consistent and correct at the setup level.version_idwiring as gt-fastapi. Pre-existing: eager loading uses unresolvedlocalesinstead ofresolved_locales, but that is outside the scope of this PR.Sequence Diagram
sequenceDiagram participant App participant initialize_gt as initialize_gt() (fastapi/flask) participant I18nManager participant RemoteLoader as create_remote_translation_loader participant CDN App->>initialize_gt: version_id, project_id, cache_url, ... initialize_gt->>initialize_gt: resolve version_id (arg || config) initialize_gt->>I18nManager: __init__(version_id=resolved_version_id, project_id, cache_url, ...) Note over I18nManager: self._version_id = version_id ✓ I18nManager->>RemoteLoader: create_remote_translation_loader(project_id, cache_url) Note over RemoteLoader: version_id NOT forwarded ✗ RemoteLoader-->>I18nManager: loader(locale) → {cache_url}/{project_id}/{locale} App->>I18nManager: get_version_id() I18nManager-->>App: self._version_id (readable only) App->>I18nManager: get_translations(locale) I18nManager->>CDN: GET {cache_url}/{project_id}/{locale} Note over CDN: version_id has no effect on URLComments Outside Diff (1)
packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.py, line 63-67 (link)version_idnot forwarded to the remote loaderversion_idis stored on the manager and exposed viaget_version_id(), but when the built-in CDN loader is constructed (lines 63–67), it only receivesproject_idandcache_url. The resulting URL is{cache_url}/{project_id}/{locale}— theversion_idis never incorporated.The docstrings in every
initialize_gt()say "Version ID for pinning translations", but with the current implementation the flag has zero effect on which translations are fetched from the CDN. Users who setversion_idexpecting deterministic/pinned translations will silently get the latest (un-pinned) version instead.If the CDN URL is expected to include the version,
create_remote_translation_loaderneeds to accept and embedversion_id:And
_remote_loader.pywould need to build the URL as:If passing
version_idto the CDN URL is intentionally deferred, the docstring should be updated to clarify that the parameter is currently only accessible viaget_version_id()for use in customload_translationscallbacks.Prompt To Fix With AI
Prompt To Fix All With AI
Last reviewed commit: "chore: bump version_..."